extensions: Read the IdeTheme and notify extension app#9182
extensions: Read the IdeTheme and notify extension app#9182srawlins wants to merge 2 commits intoflutter:masterfrom
Conversation
kenzieschmoll
left a comment
kenzieschmoll
left a comment
There was a problem hiding this comment.
I looked through the code a bit and I have an idea on how to fix this.
Add a method addThemeUpdateCallback to EditorThemeManager. You'll also need to add a method removeThemeUpdateCallback so that callbacks can be properly be cleaned up when the thing that added it is disposed.
Then from the extension iframe controller's init method, call the new addThemeUpdateCallback method on FrameworkCore.themeManager (this member will need to be made public). In this callback, call the updateTheme method like we do for changes to the preferences.darkModeEnabled setting. Modify the updateTheme method to take in the IDE theme information and pass that along in the post message data (string values only).
Where this message is received in the DevTools extension, perform this logic can be pulled out to a shared helper somewhere to update the query parameters of the DevTools extension iFrame and trigger a rebuild with the new theme.
Let me know if that makes sense or if you want to talk through this.
|
|
||
| ## DevTools Extension updates | ||
|
|
||
| TODO: Remove this section if there are not any general updates. |
There was a problem hiding this comment.
nit: please add the PR link to this release note entry (see https://github.com/flutter/devtools/blob/master/packages/devtools_app/release_notes/README.md)
| ); | ||
|
|
||
| // TODO(kenz): handle the ide theme that may be part of the query params. | ||
| setGlobal(IdeTheme, getIdeTheme()); |
There was a problem hiding this comment.
we don't need to set this again since we already set it here: https://github.com/flutter/devtools/blob/master/packages/devtools_extensions/lib/src/template/devtools_extension.dart/#L165
|
|
||
| // TODO(kenz): handle the ide theme that may be part of the query params. | ||
| setGlobal(IdeTheme, getIdeTheme()); | ||
| theme.value = ideTheme; |
There was a problem hiding this comment.
It is not clear to me how this solves the issue since we are only setting the notifier a single time and the listener in _DevToolsExtensionState.initState is actually getting added after this logic has finished.
|
Hey @srawlins! Greetings from stale PR triage! 👋 |
|
Greetings! still on my radar. |
|
Greetings from stale PR triage (the sequel)! 👋 |
|
Greetings! Yes, still is. Might squeak it in before the quarter is over. |
3 participants
Fixes #9109